fix(sns-wasm): actually overwrite existing SNS-specific upgrade path entries by Rachit2323 · Pull Request #11151 · dfinity/ic

Rachit2323 · 2026-08-16T17:41:20Z

Problem

insert_sns_specific_upgrade_path_entry is used to set a one-off emergency upgrade step for a specific SNS (e.g. "if this SNS is stuck at version A, send it to version B instead"). If you try to change that step later (e.g. because B turned out to be wrong, so it should now go to C), the function only logs that it is "being overwritten" but never actually writes the new value. The old entry stays in place, while the caller is told the request succeeded.

Fix

Actually write the new value when overwriting an existing entry, the same way the sibling function insert_upgrade_path_entry already does a few lines below it.

Testing

Added test_insert_sns_specific_upgrade_path_entry_overwrites_existing_entry, which inserts an entry, overwrites it with a different target, and checks that the second value is the one actually returned. This test fails on the old code and passes with the fix.

github-actions · 2026-08-16T23:21:12Z

Run on 396e508 URL: https://github.com/dfinity/ic/actions/runs/31978880578

github-actions

This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):

Update unreleased_changelog.md (if there are behavior changes, even if they are
non-breaking).
Are there BREAKING changes?
Is a data migration needed?
Security review?

How to Satisfy This Automatic Review

Go to the bottom of the pull request page.
Look for where it says this bot is requesting changes.
Click the three dots to the right.
Select "Dismiss review".
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:

Done.
$REASON_WHY_NO_NEED. E.g. for unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".

Brief Guide to "Externally Visible" Changes

"Externally visible behavior change" is very often due to some NEW canister API.

Changes to EXISTING APIs are more likely to be "breaking".

If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.

If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.

Reference(s)

For a more comprehensive checklist, see here.

GOVERNANCE_CHECKLIST_REMINDER_DEDUP

zeropath-ai · 2026-08-17T09:38:54Z

✅ No security or compliance issues detected. Reviewed everything up to 06aec4f.

Security Overview

🔎 Scanned files: 2 changed file(s)
🔗 Scan Link: https://zeropath.com/app/repositories/f975f1a5-0a47-4a3b-b7f0-a55609e90390?scanId=3f35b6f3-0bca-4223-9c39-d540ba289838&codeScanTypes=PrScan&tab=issues

Detected Code Changes

Change Type	Relevant files
Enhancement	► rs/nns/sns-wasm/src/sns_wasm.rs Implement and adjust SNS-specific upgrade path entry overwrite behavior ► rs/nns/sns-wasm/unreleased_changelog.md Update changelog to document replacement of SNS-specific upgrade path entries

claude · 2026-08-17T09:42:55Z

@Rachit2323 Since this touches governance-owned code, here are answers to the automated checklist so you can dismiss that review:

Changelog — this is an externally-observable behavior fix (via the governance-gated insert_upgrade_path_entries canister method), so a changelog entry is warranted. Suggested addition to rs/nns/sns-wasm/unreleased_changelog.md:

## Fixed

* `insert_upgrade_path_entries` now actually overwrites an existing
  SNS-specific upgrade path entry when called again for the same
  `(sns_governance_canister_id, from_version)` pair, instead of silently
  keeping the old target while reporting success.

Breaking? No — behavior of the broken overwrite path now matches the already-correct sibling function; no signature or type changes.
Data migration? No — purely an in-memory map-entry fix, no stored-state format change.
Security review? Already covered — this PR carries the security-review-passed label and the automated security scan reported no issues.

To clear the bot's review: use the "⋯" menu on its review, choose "Dismiss review", and paste answers to the 4 items (including the changelog snippet above, or your own wording) into the dismissal box.

Generated by Claude Code

Rachit2323 · 2026-08-17T09:48:33Z

Changelog: Done. Added a note about this fix.
Breaking changes: No. This just fixes a function to actually do what it was already supposed to do. Nothing about how you call it changed.
Data migration: No. This only affects what happens the next time someone calls this function - it doesn't touch any existing data.
Security review: Not needed. This is a small internal logic fix, no new inputs or permissions involved. The automated security scan (zeropath-ai) already came back clean.

github-actions · 2026-08-17T09:56:12Z

Run on 573426c URL: https://github.com/dfinity/ic/actions/runs/32017685403

claude · 2026-08-17T11:31:56Z

@Rachit2323 quick clarification — item 1 needs an actual commit adding that entry to rs/nns/sns-wasm/unreleased_changelog.md; a comment reply alone won't satisfy the bot's check (the PR still only shows the one file changed). Once you push that, use the "⋯" menu on the bot's review to dismiss it with your answers in the box — that's what actually clears mergeable_state.

Generated by Claude Code

claude · 2026-08-17T18:44:01Z

@Rachit2323 thanks for adding the changelog entry — one correction needed: it names the wrong function. The PR fixes insert_sns_specific_upgrade_path_entry, not insert_upgrade_path_entries (a different batch entry-point in the same file). Mind updating the changelog line to reference the right function name?

Generated by Claude Code

github-actions · 2026-08-17T19:10:24Z

Run on //rs/migration_canister:migration_canister_test URL: https://github.com/dfinity/ic/actions/runs/32058856840

github-actions · 2026-08-17T19:12:39Z

Run on eb54038 URL: https://github.com/dfinity/ic/actions/runs/32059065440

claude · 2026-08-17T19:34:56Z

@Rachit2323 heads up — master moved (a routine changelog release cut the old entries in unreleased_changelog.md), which now conflicts with this PR. To fix: merge/rebase origin/master into your branch, and in the conflict in unreleased_changelog.md's ## Fixed section, keep the section header and only your new entry (the get_available_sns_subnet one is already released, so master dropped it — no need to bring it back). No code conflict, just that one file.

Generated by Claude Code

…entries

… fix entry

github-actions · 2026-08-18T07:13:55Z

Run on 06aec4f URL: https://github.com/dfinity/ic/actions/runs/32110444482

github-actions Bot added the fix label Aug 16, 2026

cla-idx-bot Bot added the external-contributor label Aug 16, 2026

basvandijk added the security-review-passed IDX or InfraSec have concluded it's safe to run CI on the external PR. label Aug 16, 2026

Rachit2323 marked this pull request as ready for review August 17, 2026 09:38

Rachit2323 requested a review from a team as a code owner August 17, 2026 09:38

github-actions Bot added the @governance-team label Aug 17, 2026

github-actions Bot requested changes Aug 17, 2026

View reviewed changes

Rachit2323 force-pushed the fix/sns-wasm-upgrade-path-overwrite branch from 573426c to f901320 Compare August 17, 2026 18:41

Rachit2323 added 3 commits August 18, 2026 01:07

fix(sns-wasm): actually overwrite existing SNS-specific upgrade path …

57c62e8

…entries

changelog: note the SNS-specific upgrade path overwrite fix

a87eaa5

changelog: correct the function name in the SNS-specific upgrade path…

06aec4f

… fix entry

Rachit2323 force-pushed the fix/sns-wasm-upgrade-path-overwrite branch from eb54038 to 06aec4f Compare August 17, 2026 19:45

Navigation Menu

Uh oh!

fix(sns-wasm): actually overwrite existing SNS-specific upgrade path entries - #11151

fix(sns-wasm): actually overwrite existing SNS-specific upgrade path entries#11151
Rachit2323 wants to merge 3 commits into
dfinity:masterfrom
Rachit2323:fix/sns-wasm-upgrade-path-overwrite

Rachit2323 commented Aug 16, 2026

Uh oh!

github-actions Bot commented Aug 16, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

zeropath-ai Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

Rachit2323 commented Aug 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Rachit2323 commented Aug 16, 2026

Problem

Fix

Testing

Uh oh!

github-actions Bot commented Aug 16, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

How to Satisfy This Automatic Review

Brief Guide to "Externally Visible" Changes

Reference(s)

Uh oh!

zeropath-ai Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

Rachit2323 commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zeropath-ai Bot commented Aug 17, 2026 •

edited

Loading

Rachit2323 commented Aug 17, 2026 •

edited

Loading